Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActionList semantics re-introduce #3485

Closed
wants to merge 4 commits into from

Conversation

broccolinisoup
Copy link
Member

@broccolinisoup broccolinisoup commented Jul 5, 2023

Context

ActionList semantic github/accessibility-audits#2939 PRs were reverted because they were causing tests to fail at github/github. This PR re-introduces them (except 1 PR that is not stricken through) with some test and usage fixes required in the upstream dotcom.

What is changed

1) Update PRC ActionList implementation to have similar semantics to PVC.

Deprecating ActionList.Group is not a final decision - we are exploring remediating the semantics while keeping the API the same

  • Deprecated ActionList.Group.
  • Adds ActionList.Heading to be used for labelling children in an ActionList.

2) Render ActionList.Item as button

The ActionList.Item content should render an interactive button. Exceptions are when the outermost element of the ActionList.Item is an interactive element. Examples of this include NavList (where ActionList.Items can be render as a button) and in ActionMenu, where the top-level element is made interactive with the menuitem role.

github/github fixes

Integration PR: https://github.com/github/github/pull/278891

  1. There are ActionList.Item usages that has Link component as a child. GH search (Github stuff only link) and with the new change it generates an inaccessible markup like:
<button> 👉🏻 `ActionList.Item` now is rendered as button 
  <a href="..."> link text </a>
</button>

I refactored these usages to use ActionList.LinkItem commit reference (GitHub stuff only link)

  1. There are ActionList.Item usages that has role=button attribute and now ActionList.Item also returns as button, first the role attribute becomes unnecessary and it fails some tests that looks for buttons getByRole('button') because it returns multiple.GH search (Github stuff only link)

I removed the role="button" attribute from these usages.

Screenshots

Please provide before/after screenshots for any visual changes

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Changes are SSR compatible
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@broccolinisoup broccolinisoup added the skip changeset This change does not need a changelog label Jul 5, 2023
@changeset-bot
Copy link

changeset-bot bot commented Jul 5, 2023

⚠️ No Changeset found

Latest commit: 34c738b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

size-limit report 📦

Path Size
dist/browser.esm.js 127.87 KB (+25.6% 🔺)
dist/browser.umd.js 128.11 KB (+25.17% 🔺)

@github-actions github-actions bot temporarily deployed to storybook-preview-3485 July 5, 2023 03:09 Inactive
@primer primer bot temporarily deployed to github-pages July 5, 2023 03:10 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3485 July 5, 2023 03:10 Inactive
@broccolinisoup broccolinisoup temporarily deployed to github-pages July 5, 2023 03:24 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3485 July 5, 2023 03:25 Inactive
@broccolinisoup broccolinisoup marked this pull request as ready for review July 7, 2023 04:31
@broccolinisoup broccolinisoup requested review from a team and langermank July 7, 2023 04:31
@broccolinisoup broccolinisoup marked this pull request as draft July 7, 2023 04:32
@broccolinisoup broccolinisoup removed the request for review from langermank July 7, 2023 04:32
@broccolinisoup
Copy link
Member Author

We are re-introducing the remediations iteratively and without any changes in the API. Please see my comment on the issue for more details and the active PRs. https://github.com/github/accessibility-audits/issues/2939#issuecomment-1635386101 Therefore, I am closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip changeset This change does not need a changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants